Document the parallel_state deprecation and its replacement - #6257
Open
Connor-XY wants to merge 1 commit into
Open
Document the parallel_state deprecation and its replacement#6257Connor-XY wants to merge 1 commit into
Connor-XY wants to merge 1 commit into
Conversation
Contributor
Author
|
Part of #6307 — tracking issue for the |
megatron.core.parallel_state holds the process groups for a single global parallel grid, and is being replaced by explicit passing via ProcessGroupCollection. Nothing in the code said so: the module contained one occurrence of the string 'deprecat', referring to something else, and use_mpu_process_groups was documented as 'Use the default process groups from parallel_state' -- reading as a feature rather than a temporary shim. A contributor recently swapped a direct accessor for that shim believing it was a migration; it reads the same global state. Documentation only, no behavior change: - mark 53 tier-1/tier-2 accessors deprecated, naming the replacement and the independent-grid failure mode - state the deprecation, the new-feature ban and the bug-fix carve-out in the module docstring - rewrite use_mpu_process_groups to say it is a compatibility shim and not a migration target - add docs/developer/parallel-state-deprecation.md and a contribute.md section The one non-docstring hunk is black reformatting a RankGenerator assert that already violated the pinned formatter on main; the linting job runs black over every changed file, so a PR touching this file has to carry it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY
force-pushed
the
yx/pstate-p0-mark-deprecation
branch
from
August 19, 2026 19:31
5fee2db to
1715e47
Compare
Connor-XY
marked this pull request as ready for review
August 19, 2026 19:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes the
parallel_statedeprecation legible from the code. Documentation only — no behavior change.megatron.core.parallel_stateholds the process groups for a single, global parallel grid, and Megatron-Core is migrating to explicit passing viaProcessGroupCollection. Nothing in the code said so:"deprecat"inparallel_state.py(2692 lines)DeprecationWarningonset_virtual_pipeline_model_parallel_rank, not the group accessors"deprecat"inprocess_groups_config.pyuse_mpu_process_groupsdocstringDeprecationWarningon any accessorThat gap has a cost. In #5916 a contributor replaced
parallel_state.get_tensor_model_parallel_group()withProcessGroupCollection.use_mpu_process_groups()believing it was a migration; both read the same global state, so it was a lateral move. The author had no way to know from the code.This matters beyond tidiness. Three gradient-counting bugs — #5916 (merged), #6080, #6099 — trace to the same mechanism. #6099 states it directly: "gradient-norm duplicate filtering fell back to the global TP group, incorrectly excluding some expert parameters when ETP differed from TP."
Changes
parallel_state.py: the deprecation, the new-feature ban, the bug-fix carve-out, and thatuse_mpu_process_groups()is not a migration target.use_mpu_process_groupsdocstring to say it is a compatibility shim.docs/developer/parallel-state-deprecation.md— tier table, do/don't examples, reviewer guidance — plus a section incontribute.md.The tiering encodes @hao's guidance from #megatron-core-developments: existing usage may stay in bug fixes; new features must not use it.
Verification
AST-verified documentation-only: 99 functions before and after, and blanking every docstring makes the two trees compare equal.
One hunk is not a docstring.
black26.3.0 — the version pinned inuv.lock— wants to collapse aRankGeneratorassert that already fails the check onmain. The linting job runsblackover every changed file, so a PR touchingparallel_state.pyhas to carry the reformat.Rebased onto
mainat 93ebf8d. The 6 GTP-remat accessors added while this was in flight (get_gtp_weight_remat_*,get_expert_gtp_weight_remat_*) are marked too, which is why the count moved from 47 to 53.Part of #6307 — tracking issue for the
parallel_statedeprecation, with the landing order and what remains.Contribution process
Pre-checks